-
Notifications
You must be signed in to change notification settings - Fork 942
Support for payload signing + chunked encoding in DefaultAwsV4HttpSigner #6466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This commit adds support for SigV4 signing of async request payloads. In addition this commit moves the support for trailing checksums from HttpChecksumStage to the V4 signer implementation; this puts it in line with how sync chunked bodies are already handled.
- Rename to computeAndMoveContentLength - Rename variable to chunkedPayload - Fix javadocs - Sign payload only if non-null
Combine these two steps to improve performance by reducing memory allocations.
fcda677
to
488b723
Compare
3a9cb34
to
e52ea62
Compare
|
...aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/util/SignerUtils.java
Outdated
Show resolved
Hide resolved
...ain/java/software/amazon/awssdk/http/auth/aws/internal/signer/AwsChunkedV4PayloadSigner.java
Show resolved
Hide resolved
...ain/java/software/amazon/awssdk/http/auth/aws/internal/signer/AwsChunkedV4PayloadSigner.java
Outdated
Show resolved
Hide resolved
...ain/java/software/amazon/awssdk/http/auth/aws/internal/signer/AwsChunkedV4PayloadSigner.java
Show resolved
Hide resolved
...c/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java
Show resolved
Hide resolved
...are/amazon/awssdk/http/auth/aws/internal/signer/chunkedencoding/ChunkedEncodedPublisher.java
Outdated
Show resolved
Hide resolved
* and/or trailers representing trailing headers with their signature at the end. | ||
*/ | ||
@SdkInternalApi | ||
public final class AwsChunkedV4aPayloadSigner implements V4aPayloadSigner { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to support signAsync in this class as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I will follow up with a separate PR for that; same for DefaultAwsCrtV4aHttpSigner
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, but as soon as we remove the switching logic in HttpChecksumStage.java, sigv4a streaming operations will be broken, right? since it doesn't do AWS chunked encoding for async
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♂️ Ah I think you are right. In that case I can merge this back to the feature branch, and follow up with that. This PR is big enough as it is
...c/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/HttpChecksumStage.java
Show resolved
Hide resolved
- Update variable naming - Throw if decoded content length not present - Throw if input request doesn't have content-length header
Motivation and Context
Add support for payload signing of async streaming requests signed with SigV4 using default
AwsV4HttpSigner
(usingAwsV4HttpSigner.create()
). Note, requests using thehttp
URI scheme will not be signed regardless of the value ofAwsV4FamilyHttpSigner.PAYLOAD_SIGNING_ENABLED
to remain consistent with existing behavior. This may change in a future release.Modifications
Testing
Screenshots (if appropriate)
Types of changes
Checklist
mvn install
succeedsscripts/new-change
script and following the instructions. Commit the new file created by the script in.changes/next-release
with your changes.License